fix: return optional total balance in list apps response for subwallets#2057
fix: return optional total balance in list apps response for subwallets#2057im-adithya wants to merge 2 commits intomasterfrom
Conversation
📝 WalkthroughWalkthroughAdds total subwallet balance computation and returns it in the ListApps API response. Subwallet filtering logic was refactored to handle tri-state filtering. Frontend updated to request subwallets and consume totalCount and totalBalance. Changes
Sequence DiagramsequenceDiagram
participant FE as Frontend (SubwalletList)
participant API as API (ListApps)
participant DB as Database (Queries)
FE->>API: ListApps(subWallets: true)
API->>API: Apply SubWallets tri-state filtering
API->>DB: GetTotalSubwalletBalance(tx)
DB->>DB: Select subwallet app IDs (metadata.app_store_app_id)
DB->>DB: SUM INCOMING where state=SETTLED
DB->>DB: SUM OUTGOING (amount+fee+reserve) where state IN (SETTLED,PENDING)
DB-->>API: Return net balance (received - spent)
API-->>FE: ListAppsResponse { Apps, TotalCount, TotalBalance }
FE->>FE: Render list and display totalCount & totalBalance
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@api/api.go`:
- Around line 530-535: The call to queries.GetTotalSubwalletBalance currently
assumes a single int64 return and masks DB errors by setting totalBalance to 0;
change the call-site in the block that checks filters.SubWallets to handle the
new signature (int64, error) from GetTotalSubwalletBalance: call
totalBalanceMsat, err := queries.GetTotalSubwalletBalance(api.db), on error log
the error via the existing logger and return the error (or otherwise degrade by
leaving totalBalance nil), and only set totalBalance = &totalBalanceMsat when
err == nil; update references to totalBalance accordingly to avoid exposing a
zero balance on DB failures.
In `@db/queries/get_total_subwallet_balance.go`:
- Around line 10-33: GetTotalSubwalletBalance currently ignores database errors
from the two Scan calls (Scan(&received) and Scan(&spent)), so change its
signature to return (int64, error), capture the returned *gorm.DB from each Scan
(e.g., res := tx.Table(...).Scan(&received)), check res.Error and return 0, err
on failure, and only compute and return received.Sum - spent.Sum with nil error
when both queries succeed; update the caller in api/api.go to handle the new
(int64, error) result accordingly.
🧹 Nitpick comments (1)
db/queries/get_total_subwallet_balance_test.go (1)
25-59: Consider checking errors fromsvc.DB.Save()calls.Multiple
svc.DB.Save(...)calls (Lines 25, 31, 39, 49, 59) don't assert on the returned error. A silent save failure would cause a misleading test result. Usingrequire.NoError(t, svc.DB.Save(...).Error)would make failures easier to diagnose.
| func GetTotalSubwalletBalance(tx *gorm.DB) int64 { | ||
| subwalletAppIDsQuery := tx.Model(&db.App{}). | ||
| Select("id"). | ||
| Where(datatypes.JSONQuery("metadata").Equals(constants.SUBWALLET_APPSTORE_APP_ID, "app_store_app_id")) | ||
|
|
||
| var received struct { | ||
| Sum int64 | ||
| } | ||
| tx. | ||
| Table("transactions"). | ||
| Select("SUM(amount_msat) as sum"). | ||
| Where("app_id IN (?) AND type = ? AND state = ?", subwalletAppIDsQuery, constants.TRANSACTION_TYPE_INCOMING, constants.TRANSACTION_STATE_SETTLED). | ||
| Scan(&received) | ||
|
|
||
| var spent struct { | ||
| Sum int64 | ||
| } | ||
| tx. | ||
| Table("transactions"). | ||
| Select("SUM(amount_msat + fee_msat + fee_reserve_msat) as sum"). | ||
| Where("app_id IN (?) AND type = ? AND (state = ? OR state = ?)", subwalletAppIDsQuery, constants.TRANSACTION_TYPE_OUTGOING, constants.TRANSACTION_STATE_SETTLED, constants.TRANSACTION_STATE_PENDING). | ||
| Scan(&spent) | ||
|
|
||
| return received.Sum - spent.Sum |
There was a problem hiding this comment.
Missing error handling on DB queries.
Both Scan(&received) (Line 22) and Scan(&spent) (Line 31) ignore the returned *gorm.DB result, so any query failure is silently swallowed — the function would return 0 (or a partial result) without the caller knowing. Consider returning an (int64, error) tuple, or at minimum logging errors.
Proposed fix: return errors to caller
-func GetTotalSubwalletBalance(tx *gorm.DB) int64 {
+func GetTotalSubwalletBalance(tx *gorm.DB) (int64, error) {
subwalletAppIDsQuery := tx.Model(&db.App{}).
Select("id").
Where(datatypes.JSONQuery("metadata").Equals(constants.SUBWALLET_APPSTORE_APP_ID, "app_store_app_id"))
var received struct {
Sum int64
}
- tx.
+ if err := tx.
Table("transactions").
Select("SUM(amount_msat) as sum").
Where("app_id IN (?) AND type = ? AND state = ?", subwalletAppIDsQuery, constants.TRANSACTION_TYPE_INCOMING, constants.TRANSACTION_STATE_SETTLED).
- Scan(&received)
+ Scan(&received).Error; err != nil {
+ return 0, err
+ }
var spent struct {
Sum int64
}
- tx.
+ if err := tx.
Table("transactions").
Select("SUM(amount_msat + fee_msat + fee_reserve_msat) as sum").
Where("app_id IN (?) AND type = ? AND (state = ? OR state = ?)", subwalletAppIDsQuery, constants.TRANSACTION_TYPE_OUTGOING, constants.TRANSACTION_STATE_SETTLED, constants.TRANSACTION_STATE_PENDING).
- Scan(&spent)
+ Scan(&spent).Error; err != nil {
+ return 0, err
+ }
- return received.Sum - spent.Sum
+ return received.Sum - spent.Sum, nil
}The caller in api/api.go (Lines 532-535) would need a corresponding update to handle the error.
🤖 Prompt for AI Agents
In `@db/queries/get_total_subwallet_balance.go` around lines 10 - 33,
GetTotalSubwalletBalance currently ignores database errors from the two Scan
calls (Scan(&received) and Scan(&spent)), so change its signature to return
(int64, error), capture the returned *gorm.DB from each Scan (e.g., res :=
tx.Table(...).Scan(&received)), check res.Error and return 0, err on failure,
and only compute and return received.Sum - spent.Sum with nil error when both
queries succeed; update the caller in api/api.go to handle the new (int64,
error) result accordingly.
There was a problem hiding this comment.
@rolznz we don't do this for any query in db/queries, I think this should be fine?
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
@im-adithya hmm, I think we should add it actually, especially for the other queries - that's my fault for not adding it. I'll add an issue.
There was a problem hiding this comment.
Added here, will open a new PR to add it on other db queries too 👌
2 participants
Fixes #2031
Summary by CodeRabbit
New Features
Bug Fixes / Accuracy
Tests